From 4d58457784386efffa508fd65e5914d76e0f797f Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 25 Mar 2015 17:29:31 -0700
Subject: [PATCH] Added slave/master fallback logic in Revision

* This is a more specific form of the logic removed in 3c2bc32ae1.
  It does not suffer the problem of causing constant master DB
  queries due to a bad template reference or such.
* It will use the master if writes from the current thread
  are pending or were recently committed. This deals with the
  common problem of code that needs to read things it just wrote,
  such as diffs on rollback or edit hooks.
* This commit reverts 8624e261f by making the hack obsolete.

Bug: T93866
Bug: T94407
Change-Id: Ib9ecb75e1236e767bdc86d124d5e22a03ae0fb5f
---
 includes/Revision.php              | 21 ++++++++++++++--
 includes/db/Database.php           |  2 ++
 includes/db/LoadBalancer.php       | 39 ++++++++++++++++++++++++++++--
 includes/diff/DifferenceEngine.php |  6 -----
 4 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/includes/Revision.php b/includes/Revision.php
index 356cd32ce9..a13499a632 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -297,7 +297,10 @@ class Revision implements IDBAccessObject {
 	}
 
 	/**
-	 * Given a set of conditions, fetch a revision.
+	 * Given a set of conditions, fetch a revision
+	 *
+	 * This method is used then a revision ID is qualified and
+	 * will incorporate some basic slave/master fallback logic
 	 *
 	 * @param array $conditions
 	 * @param int $flags (optional)
@@ -305,10 +308,24 @@ class Revision implements IDBAccessObject {
 	 */
 	private static function newFromConds( $conditions, $flags = 0 ) {
 		$db = wfGetDB( ( $flags & self::READ_LATEST ) ? DB_MASTER : DB_SLAVE );
+
 		$rev = self::loadFromConds( $db, $conditions, $flags );
+		// Make sure new pending/committed revision are visibile later on
+		// within web requests to certain avoid bugs like T93866 and T94407.
+		if ( !$rev
+			&& !( $flags & self::READ_LATEST )
+			&& wfGetLB()->getServerCount() > 1
+			&& wfGetLB()->hasOrMadeRecentMasterChanges()
+		) {
+			$flags = self::READ_LATEST;
+			$db = wfGetDB( DB_MASTER );
+			$rev = self::loadFromConds( $db, $conditions, $flags );
+		}
+
 		if ( $rev ) {
 			$rev->mQueryFlags = $flags;
 		}
+
 		return $rev;
 	}
 
@@ -1481,9 +1498,9 @@ class Revision implements IDBAccessObject {
 	 * @return string|bool The revision's text, or false on failure
 	 */
 	protected function loadText() {
-
 		// Caching may be beneficial for massive use of external storage
 		global $wgRevisionCacheExpiry, $wgMemc;
+
 		$textId = $this->getTextId();
 		$key = wfMemcKey( 'revisiontext', 'textid', $textId );
 		if ( $wgRevisionCacheExpiry ) {
diff --git a/includes/db/Database.php b/includes/db/Database.php
index b3c81f9bf6..cbd8be5bd0 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3611,6 +3611,7 @@ abstract class DatabaseBase implements IDatabase {
 			$this->runOnTransactionPreCommitCallbacks();
 			$this->doCommit( $fname );
 			if ( $this->mTrxDoneWrites ) {
+				$this->mDoneWrites = microtime( true );
 				$this->getTransactionProfiler()->transactionWritingOut(
 					$this->mServer, $this->mDBname, $this->mTrxShortId );
 			}
@@ -3691,6 +3692,7 @@ abstract class DatabaseBase implements IDatabase {
 		$this->runOnTransactionPreCommitCallbacks();
 		$this->doCommit( $fname );
 		if ( $this->mTrxDoneWrites ) {
+			$this->mDoneWrites = microtime( true );
 			$this->getTransactionProfiler()->transactionWritingOut(
 				$this->mServer, $this->mDBname, $this->mTrxShortId );
 		}
diff --git a/includes/db/LoadBalancer.php b/includes/db/LoadBalancer.php
index 4e0a5b9cc3..48c636e484 100644
--- a/includes/db/LoadBalancer.php
+++ b/includes/db/LoadBalancer.php
@@ -1022,8 +1022,7 @@ class LoadBalancer {
 	}
 
 	/**
-	 * Determine if there are any pending changes that need to be rolled back
-	 * or committed.
+	 * Determine if there are pending changes in a transaction by this thread
 	 * @since 1.23
 	 * @return bool
 	 */
@@ -1044,6 +1043,42 @@ class LoadBalancer {
 		return false;
 	}
 
+	/**
+	 * Get the timestamp of the latest write query done by this thread
+	 * @since 1.25
+	 * @return float|bool UNIX timestamp or false
+	 */
+	public function lastMasterChangeTimestamp() {
+		$lastTime = false;
+		// Always 0, but who knows.. :)
+		$masterIndex = $this->getWriterIndex();
+		foreach ( $this->mConns as $conns2 ) {
+			if ( empty( $conns2[$masterIndex] ) ) {
+				continue;
+			}
+			/** @var DatabaseBase $conn */
+			foreach ( $conns2[$masterIndex] as $conn ) {
+				$lastTime = max( $lastTime, $conn->lastDoneWrites() );
+			}
+		}
+		return $lastTime;
+	}
+
+	/**
+	 * Check if this load balancer object had any recent or still
+	 * pending writes issued against it by this PHP thread
+	 *
+	 * @param float $age How many seconds ago is "recent" [defaults to mWaitTimeout]
+	 * @return bool
+	 * @since 1.25
+	 */
+	public function hasOrMadeRecentMasterChanges( $age = null ) {
+		$age = ( $age === null ) ? $this->mWaitTimeout : $age;
+
+		return ( $this->hasMasterChanges()
+			|| $this->lastMasterChangeTimestamp() > microtime( true ) - $age );
+	}
+
 	/**
 	 * @param mixed $value
 	 * @return mixed
diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index de8dd43bca..77bbd36ab8 100644
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -1233,12 +1233,6 @@ class DifferenceEngine extends ContextSource {
 		// Load the new revision object
 		if ( $this->mNewid ) {
 			$this->mNewRev = Revision::newFromId( $this->mNewid );
-
-			if ( !$this->mNewRev && wfGetLB()->getServerCount() > 1 ) {
-				// Try harder… This is being hit after a rollback where we show the
-				// diff immediately after the edit happened. T93866
-				$this->mNewRev = Revision::newFromId( $this->mNewid, Revision::READ_LATEST );
-			}
 		} else {
 			$this->mNewRev = Revision::newFromTitle(
 				$this->getTitle(),
-- 
2.20.1